Skip to content

fix(desktop,daemon): keep replacement-pattern sequences verbatim in export titles - #6947

Merged
AmyShang-alt merged 1 commit into
nexu-io:mainfrom
cactusrabbit:fix/export-title-replacement-patterns
Aug 17, 2026
Merged

fix(desktop,daemon): keep replacement-pattern sequences verbatim in export titles#6947
AmyShang-alt merged 1 commit into
nexu-io:mainfrom
cactusrabbit:fix/export-title-replacement-patterns

Conversation

@cactusrabbit

Copy link
Copy Markdown
Contributor

Fixes #6795

Why

injectTitle() (both apps/desktop/src/main/pdf-export.ts and apps/desktop/src/main/artifact-export.ts) passes the user-derived title as the string replacement of String.prototype.replace, so ECMA-262 GetSubstitution expands $$, $&, $``, and $'inside it. Artifact titles containing those sequences were corrupted in exported PDFs/images β€” dropped$, broken <title>tags, and duplicated document tails.assembleExample()in the daemon'sstatic-resource.ts` interpolates skill-derived slides and titles through the same mechanism.

Changes

  • apps/desktop/src/main/pdf-export.ts β€” use a function replacement in injectTitle (matching the sibling injectBaseHref pattern).
  • apps/desktop/src/main/artifact-export.ts β€” same change in its injectTitle.
  • apps/daemon/src/routes/static-resource.ts β€” harden both interpolations in assembleExample (slides + title) the same way.
  • Regression tests:
    • apps/desktop/tests/main/export-title-replacement-patterns.test.ts β€” drives exportPdfFromHtml and exportArtifact with titles containing $$, $&, $``, $'` and asserts the loaded document keeps the title verbatim (HTML-escaped only) and the body intact.
    • apps/daemon/tests/assemble-example-title-patterns.test.ts β€” asserts assembleExample interpolates slides and titles verbatim.

Surface area

  • UI β€” n/a (no visual change; corrects rendered export documents)
  • Keyboard shortcut β€” none
  • CLI / env var β€” none
  • API / contract β€” none
  • Extension point β€” none
  • i18n keys β€” none
  • New top-level dependency β€” none
  • Default behavior change β€” exported documents with $-pattern titles now render correctly
  • None β€” targeted bug fix + tests

Validation

  • pnpm --filter @open-design/desktop exec vitest run -c vitest.config.ts tests/main/export-title-replacement-patterns.test.ts β€” 2 passed
  • pnpm --filter @open-design/daemon exec vitest run -c vitest.config.ts tests/assemble-example-title-patterns.test.ts β€” 1 passed
  • pnpm --filter @open-design/desktop typecheck β€” passed
  • pnpm --filter @open-design/daemon typecheck β€” passed
  • pnpm guard β€” all checks passed except a pre-existing ci.yml packaged-leaf boundary failure, untouched by this PR

…xport titles (nexu-io#6795)

String.replace expands $$, $&, $` and $' in string replacements, corrupting
user-derived artifact titles in PDF/image export and skill-derived titles in
example assembly. Switch the affected interpolations to function replacements
and add regression tests for both exporters and assembleExample.
@cactusrabbit
cactusrabbit requested a review from a team as a code owner August 16, 2026 03:43
@lefarcen

Copy link
Copy Markdown
Contributor

Hey @cactusrabbit β€” thanks for the focused fix here. Extending the hardening to the daemon-side assembleExample() path as well makes the overall direction very clear.

Heads-up: PR #6796, PR #6891, and PR #6931 are already open against issue #6795 and touch the same export-title path, so maintainers will likely consolidate around one path here rather than land parallel fixes.

@lefarcen
lefarcen requested a review from nettee August 16, 2026 03:47
@lefarcen lefarcen added size/M PR changes 100-300 lines risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps type/bugfix Bug fix needs-validation Runtime change detected; needs human or /explore agent validation. labels Aug 16, 2026
@lefarcen

Copy link
Copy Markdown
Contributor

πŸ§ͺ Queued for QA validation β€” this PR changes exported document output, so it needs a manual QA pass before merge. Nothing needed from you right now; we’ll update here once that pass is done. Thanks for the contribution.

@lefarcen lefarcen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @cactusrabbit β€” the write-up here is already easy to follow, especially the concrete Why/Changes split and the regression-test callouts. One body-level thing before pool review: since the UI box is checked, could you add a quick before/after screenshot (or exported-document capture) showing one of the $-pattern titles rendered correctly? That would complete the template without changing the rest of the description.

@nettee nettee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cactusrabbit I reviewed all five changed files and verified that the function replacements preserve literal JavaScript replacement-pattern sequences across the desktop PDF/image exporters and daemon example assembly. The focused desktop (2 tests) and daemon (1 test) regression suites pass, along with both package typechecks, the workspace typecheck, and the repository guard. This is a nicely focused fix with clear regression coverageβ€”thank you for carefully closing the loop on #6795!

πŸ” Powered by Looper Β· runner=reviewer Β· agent=codex Β· An autonomous AI dev team for your GitHub repos.

@lefarcen
lefarcen requested a review from AmyShang-alt August 16, 2026 04:02
@AmyShang-alt AmyShang-alt added validated Runtime change validated (via /explore Pass or manual QA). and removed needs-validation Runtime change detected; needs human or /explore agent validation. labels Aug 17, 2026
@AmyShang-alt

Copy link
Copy Markdown
Contributor

@cactusrabbit

Thanks for the contribution. I completed QA validation for this PR.

QA Acceptance Record

Scope:

  • Desktop PDF/image export title injection and daemon example assembly for literal JavaScript replacement-pattern sequences.

Verified:

  • Exact PR head a09f1b1.
  • Focused desktop and daemon regression suites passed (3 tests total).
  • Literal $$, $&amp;, $', and $` title sequences remained intact in Electron export validation; PDF metadata matched the supplied title.
  • Real Electron toolbar export produced a complete 10-page PDF with visible titles and intact layout.
  • Required CI checks are green.

Not verified:

  • Packaged-app export was not repeated across every supported OS.

Risks / notes:

  • The change is narrowly scoped and has direct regression coverage. No analytics or static-asset behavior changed.

Conclusion:

  • Accepted.

@AmyShang-alt
AmyShang-alt added this pull request to the merge queue Aug 17, 2026
Merged via the queue into nexu-io:main with commit c144c80 Aug 17, 2026
35 checks passed
@open-design-bot

Copy link
Copy Markdown
Contributor

πŸŽ‰ πŸ“‘ You just leveled up to Giotto

Giotto card for @cactusrabbit

πŸ“‘ ✨ Sending steady signals.

πŸ™Œ Your contributions are sending a clear signal across the network: you care about making Open Design better. Keep transmitting.

πŸ’› Thanks for helping Open Design move forward. Keep building in the open. πŸš€


πŸ“Š Rank #174 among 600+ contributors

πŸ”— Share on X (English) Β· εˆ†δΊ«εˆ° XοΌˆδΈ­ζ–‡οΌ‰

@lefarcen

Copy link
Copy Markdown
Contributor

Heads-up: PR #6931 is also still open for issue #6795. Its current head is focused on the desktop regression test, while this PR carries the desktop code changes plus the daemon hardening follow-up, so it would be good to coordinate the scopes and avoid landing overlapping fixes twice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps size/M PR changes 100-300 lines type/bugfix Bug fix validated Runtime change validated (via /explore Pass or manual QA).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Desktop PDF/image export corrupts titles containing $$, $&, $` or $' (injectTitle expands replace() patterns)

4 participants